Skip to content

Add zmalloc_aligned() and fix SPMC queue buffer alignment#3504

Merged
madolson merged 5 commits intovalkey-io:unstablefrom
sarthakaggarwal97:daily-fixes-20260414
Apr 23, 2026
Merged

Add zmalloc_aligned() and fix SPMC queue buffer alignment#3504
madolson merged 5 commits intovalkey-io:unstablefrom
sarthakaggarwal97:daily-fixes-20260414

Conversation

@sarthakaggarwal97
Copy link
Copy Markdown
Contributor

@sarthakaggarwal97 sarthakaggarwal97 commented Apr 14, 2026

The SPMC queue from #3324 needs each spmcCell to be cache-line aligned, but plain zmalloc() does not guarantee that in all build configurations.

This change introduces zmalloc_cache_aligned() and uses it for the SPMC queue buffer allocation in spmcInit().

Failing CI: https://github.com/valkey-io/valkey/actions/runs/24374139344

Worked with Codex!

@sarthakaggarwal97 sarthakaggarwal97 marked this pull request as draft April 14, 2026 16:51
@sarthakaggarwal97 sarthakaggarwal97 marked this pull request as ready for review April 14, 2026 17:03
@sarthakaggarwal97 sarthakaggarwal97 added the run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP) label Apr 14, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 14, 2026

Codecov Report

❌ Patch coverage is 92.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.42%. Comparing base (03c2d4c) to head (86961f1).
⚠️ Report is 6 commits behind head on unstable.

Files with missing lines Patch % Lines
src/zmalloc.c 80.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #3504      +/-   ##
============================================
+ Coverage     76.21%   76.42%   +0.21%     
============================================
  Files           159      159              
  Lines         81672    81695      +23     
============================================
+ Hits          62243    62439     +196     
+ Misses        19429    19256     -173     
Files with missing lines Coverage Δ
src/queues.c 99.35% <100.00%> (ø)
src/unit/test_queues.cpp 99.57% <100.00%> (+<0.01%) ⬆️
src/unit/test_zmalloc.cpp 100.00% <100.00%> (ø)
src/zmalloc.c 84.15% <80.00%> (-0.17%) ⬇️

... and 24 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sarthakaggarwal97 sarthakaggarwal97 marked this pull request as draft April 14, 2026 17:49
@sarthakaggarwal97 sarthakaggarwal97 marked this pull request as ready for review April 14, 2026 19:07
@sarthakaggarwal97
Copy link
Copy Markdown
Contributor Author

Macos Test should get stabilised by #3509

@sarthakaggarwal97 sarthakaggarwal97 changed the title Fix SPMC queue buffer allocation to guarantee spmcCell is cache-line aligned Add zmalloc_aligned() and fix SPMC queue buffer alignment Apr 16, 2026
@sarthakaggarwal97 sarthakaggarwal97 force-pushed the daily-fixes-20260414 branch 2 times, most recently from 20e266c to 83bf327 Compare April 17, 2026 03:08
Comment thread src/zmalloc.h Outdated
Comment thread src/unit/test_zmalloc.cpp Outdated
Introduce zmalloc_aligned(alignment, size) for cache-line-aligned heap
allocations with full Valkey memory tracking. On HAVE_MALLOC_SIZE platforms
it wraps posix_memalign directly. On !HAVE_MALLOC_SIZE platforms it
over-allocates and stores the raw pointer and a flag-tagged size before
the aligned region so zfree() can transparently recover the original
pointer.

Use zmalloc_aligned() in spmcInit() to guarantee each spmcCell is
cache-line aligned, fixing UBSan failures in sanitizer CI builds
which force MALLOC=libc.

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Replace the general-purpose zmalloc_aligned(alignment, size) with a
simpler zmalloc_cache_aligned(size) that always aligns to CACHE_LINE_SIZE.

This is the only alignment any caller needs, so drop:
- The !HAVE_MALLOC_SIZE manual alignment path (MSB flag, raw pointer
  storage, modified zfree/zfree_with_size/zmalloc_size)
- The alignment validation helper
- The posix_memalign wrapper

The result is ~70 fewer lines with the same practical effect.
On !HAVE_MALLOC_SIZE platforms (none that Valkey ships on), it panics
at startup rather than silently corrupting memory.

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
@sarthakaggarwal97
Copy link
Copy Markdown
Contributor Author

The test failures are not related to this change. Related to #2936 and test-ubuntu-latest ones would be fixed by #3511

@madolson madolson merged commit 5abf79e into valkey-io:unstable Apr 23, 2026
63 of 66 checks passed
sarthakaggarwal97 added a commit to sarthakaggarwal97/valkey that referenced this pull request Apr 23, 2026
…3504)

The SPMC queue from valkey-io#3324 needs each `spmcCell` to be cache-line
aligned, but plain `zmalloc()` does not guarantee that in all build
configurations.

This change introduces `zmalloc_cache_aligned()` and uses it for the
SPMC queue buffer allocation in `spmcInit()`.

Failing CI: https://github.com/valkey-io/valkey/actions/runs/24374139344

---------

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants